-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ARC auto-sync architecture #2570
Conversation
@Rot127, fuzzing, as I expected, shows that those instructions that are not yet supported for ARC in llvm lead to incorrect decoding and program crash. llvm-mc from the official github also fails with an error. Should we do something now or ignore it until llvm updates? |
You can fix it in our fork of LLVM. And open an issue in LLVM with a reference to the fix commit. They can cherry-pick it. If you have the time, you can also open a PR with the fix yourself. |
The problem is solved now. The DecodeGPR32RegisterClass return value was not checked (in llvm disassembler also) |
aa93895
to
e2fb2d7
Compare
e2fb2d7
to
06afa37
Compare
Do you consider this done? Then I would review it this week. |
Yes. I think it is done now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Continue later (at arc.h
). But so far looks very good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a very straight forward review! Excellent job, well done!
Please add ARC
in cs_v6_release_guide.md
und New features
.
Simple two liner is enough. Just please mention about the memory operands.
Otherwise, please address the other small requests.
Regarding the not yet implemented memory operand type: Do you want to add them? It complicates the details a bit. But on the other hand there seems only the MemOperandRI
type. So not too much work I think? Or was there another problem?
Nonetheless, we can merge it like this. But we should have them before v6 Gold.
cc @XVilka @kabeor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Almost done. Just those few nitpicks.
Also, please mark the suggestions as resolved when done.
suite/auto-sync/src/autosync/cpptranslator/patches/BadConditionCode.py
Outdated
Show resolved
Hide resolved
suite/auto-sync/src/autosync/cpptranslator/patches/BadConditionCode.py
Outdated
Show resolved
Hide resolved
@R33v0LT there are some conflicts now, could you please rebase it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! It's amazing!
Rebased with #2614 |
* Add ARC files * Added ARC files for successful compilation * Refactor ARC files * Add ARC c/cs tests * Add ARC python test/bindings * Add ARC to CI/CD * Avoid omitting parameter names * Update cs files * Fix ARC bugs * Update ARC python bindings * Refactor and update ARC test files * Add detail flag to arc test * Fix ARC test problems * Fix ARCMapping compile error * Replace __CHAR_BIT__ to CHAR_BIT * Add credits and ARC info * Update ARC to match the latest next * Python formatting * Remove asserts on 'Unknown condition code passed' * Add ARC to some more documentation * Add ARC to Targets constants * Add ARC support to llvm-tblgen * Replace asserts & add Expr handling * Check DecodeGPR32RegisterClass return value * Fix fieldFromInstruction patch * Refactor ARC * Reformat python files * Fix incorrect import * Update inc files and insn names * Update python files * Disable AArch64 Linux wheel build due to #2615. --------- Co-authored-by: R33v0LT <[email protected]>
Closed with #2614 |
Your checklist for this pull request
Detailed description
This PR adds support for the ARC architecture in capstone. It is worth noting that this architecture is experimental in lvm, that is, full support for all instructions is not fully implemented. Also, MC tests in llvm (like llvm_root/llvm/test/MC/Disassembler/ARC/ldst.txt ) have a format different from other architectures, which is why tests auto-generated by MCUpdater.py does not add instruction bytes to yaml. I think this format will change when ARC is fully supported
To successfully pass the autosync test, a merge of the following PR is required: capstone-engine/llvm-capstone#69
Test plan
There are no differences in the way capstone is built with the ARC architecture
MC tests were generated by MCUpdater. Only instruction bytes are manually added. All other tests are added manually
At the moment, all tests are passing successfully, there are CI/CD results to confirm
Closing issues
None